Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add option to run templates in current AssemblyLoadContext #170

Closed
wants to merge 1 commit into from
Closed

Add option to run templates in current AssemblyLoadContext #170

wants to merge 1 commit into from

Conversation

ajcvickers
Copy link

Fixes #169

Happy to refactor and/or add tests as needed.

@ajcvickers
Restore option to load assembly into the current AppDomain
5f31951 
Fixes mono#169

Happy to refactor and/or add tests as needed.
@mhutch
Copy link
Member

mhutch commented Dec 6, 2023 •
edited
Loading

I'd rather not change behavior in a point release so at the very least the existing parameterless method should preserve the behavior. However, it might not be necessary to add this overload at all.

There's already a mechanism for controlling whether an AppDomain is used - you can return null from ProvideTemplatingAppDomain, which is what default implementation does. It appears that EF has overridden it to return AppDomain.CurrentDomain. You may be able to fix it by removing this override https://github.com/dotnet/efcore/blob/e84696d990aefd28a7d74eddae0d0cf934320f9f/src/EFCore.Design/Scaffolding/Internal/TextTemplatingEngineHost.cs#L174

We could certainly improve usability by altering this method to detect if the AppDomain is AppDomain.CurrentDomain and in that case either throw an ArgumentException or use the non-AppDomain codepath: https://github.com/mono/t4/blob/release/v2.3.0/Mono.TextTemplating/Mono.TextTemplating/CompiledTemplate.cs#L85

mhutch added a commit that referenced this pull request Dec 6, 2023
@mhutch
Don't use AppDomain codepath for CurrentDomain
9fb6cc2 
Template hosts should return null from ProvideTemplatingAppDomain if they
don't want to use an AppDomain, but check for AppDomain.CurrentDomain
and shortcircuit that as well.

Improves usability issue discovered in #170
mhutch added a commit that referenced this pull request Dec 6, 2023
@mhutch
Don't use AppDomain codepath for CurrentDomain
145334b 
Template hosts should return null from ProvideTemplatingAppDomain if they
don't want to use an AppDomain, but check for AppDomain.CurrentDomain
and shortcircuit that as well.

Improves usability issue discovered in #170
@mhutch mhutch mentioned this pull request Dec 6, 2023
Merged
@ajcvickers
Copy link
Author

@mhutch ProvideTemplatingAppDomain is never called by Mono.TextTemplating when running the EF code. It looks like this is only used when FEATURE_APPDOMAINS is set, which is only on .NET Framework.

@mhutch
Copy link
Member

mhutch commented Dec 7, 2023

Ah, sorry! Yes, there's no way to disable the use of AssemblyLoadContext. I thought its assembly loading was more robust than the AppDomain version but it looks like it has issues, so there should definitely be a way to opt out.

@mhutch mhutch changed the title Restore option to load assembly into the current AppDomain Add option to run templates in current AssemblyLoadContext Dec 7, 2023
@ajcvickers ajcvickers closed this Aug 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Restore option to load assembly into the current AppDomain
2 participants
@ajcvickers @mhutch